Move to new API for GetSuggestedActions

[mavasani]

Editor team recently added a new GetSuggestedActions API overload that allows providing a more detailed operation scope message. This PR moves us to that API for Roslyn suggested actions. We now display the name of the executing analyzer/fixer/refactoring when user hits Ctrl + Dot to bring up the light bulb. This should help the users identify the slow extension and help us in improving performance of our analyzers/fixers/refactorings.

[CyrusNajmabadi]

This is definitely interesting. But my preference woudl be more than when i'm invoking a code-action i get good feedback about what is going on. We used to support that, but now all i see is something vague like "applying code action to 'project'" which is effectively useless.

[CyrusNajmabadi]

done with pass.

[mavasani]

But my preference woudl be more than when i'm invoking a code-action i get good feedback about what is going on. We used to support that, but now all i see is something vague like "applying code action to 'project'" which is effectively useless.

Thanks, that would be a separate feature request. This PR mainly aims at improving the messaging when computing diagnostics + code actions for light bulb, so we can tackle some of the delays we are seeing light bulb showing up after Ctrl + Dot.

[sharwell]

@mavasani Can you submit the NRT changes to master, and then we'll update this to a simpler form?

[mavasani]

Can you submit the NRT changes to master, and then we'll update this to a simpler form?

Seems like lot of unnecessary duplicate work… the NRT changes are minimal, so is there a big concern in doing it within this PR? I believe it is best to keep enabling nullable as and when we update/fix code. Splitting into separate PR makes sense for large refactoring where nullable is enabled across large number of files/projects...

[CyrusNajmabadi]

Thanks, that would be a separate feature request.

Agreed. Though my general feedback is: can we fix our existing bad behavior (and regressions) in this space prior to doing more feature work?

[mavasani]

Seems like we still want to know when analyzers are running outside of lightbulb scenarios, just not for cases where the results are already computed and available from a cache.

Yes, but that should probably come at some other place. This code path does not guarantee analyzers are executed, their results might be used from the cache on this path.

[CyrusNajmabadi]

I'm on the fence about the nullable changes. I think ther'es enough value here to keep in this PR. but it definitely added noise. However, as the king of making noisy chnages, it would be hypocritical of me to protest when i'm reviewing myself :D

[olegtk]

In terms of using new LightBulb API looks good.

[mavasani]

@sharwell @CyrusNajmabadi Any further feedback? thanks!

[mavasani]

@jasonmalinowski Test failures in this PR seem to be coming from the inferred indentation kicking in at

roslyn/src/EditorFeatures/Core/Implementation/Formatting/InferredIndentationDocumentOptionsProviderFactory.cs

Lines 92 to 98 in aeabd60

	private bool TryGetOptionForBuffer(ITextBuffer textBuffer, OptionKey option, out object? value)
	{
	if (option.Option == FormattingOptions.UseTabs)
	{
	value = !_indentationManagerService.UseSpacesForWhitespace(textBuffer, explicitFormat: false);
	return true;
	}

. This seems to be likely due to the fact that we are moving to a newer VS platform assemblies. Failing tests are:

1. Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.DocumentationComments.DocumentationCommentTests.TestPressingEnter_Indentation5_UseTabs
2. Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.DocumentationComments.DocumentationCommentTests.TestOpenLineBelow4_Tabs
3. Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.DocumentationComments.DocumentationCommentTests.TestOpenLineAbove4_Tabs
4. Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.DocumentationComments.DocumentationCommentTests.TestOpenLineAbove4_Tabs
5. Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.DocumentationComments.DocumentationCommentTests.TestOpenLineBelow4_Tabs
6. Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.DocumentationComments.DocumentationCommentTests.PressingEnter_Indentation5_UseTabs

I can do one of the following to resolve this (in order of my preference):

1. Skip the failing tests with a tracking bug assigned to you
2. Update the test to not have mixed tabs/spaces so inferred indentation does not kick in
3. Add support (possibly an option) to allow disabling inferred indentation for unit tests

Let me know what you prefer. I took the first approach and filed #39967 - let me know if you feel otherwise.

[mavasani]

@dotnet/roslyn-infrastructure What is the version of Visual Studio being used for our integration tests? Have we moved to latest 16.4 public preview or are the integration tests still running on 16.3?